Skip to content

[improve][function] Introduced protections against deserialization attacks#22723

Open
aditchawdhary wants to merge 1 commit intoapache:masterfrom
aditchawdhary:functions-jst
Open

[improve][function] Introduced protections against deserialization attacks#22723
aditchawdhary wants to merge 1 commit intoapache:masterfrom
aditchawdhary:functions-jst

Conversation

@aditchawdhary
Copy link

@aditchawdhary aditchawdhary commented May 16, 2024 •
edited
Loading

Motivation

This change hardens Java deserialization operations against attack. Even a simple operation like an object deserialization is an opportunity to yield control of your system to an attacker. In fact, without specific, non-default protections, any object deserialization call can lead to arbitrary code execution.

Modifications

I have added pixee java security toolkit as a dependency, and in pulsar functions in the the Serialization/ Deserialization file I have added ObjectInputFilters.enableObjectFilterIfUnprotected to the object input stream.

Motivation

Modifications

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions
Copy link

github-actions bot commented May 16, 2024

@Khac Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@github-actions github-actions bot added doc-label-missing doc-required Your PR changes impact docs and you will update later. doc-not-needed Your PR changes do not impact docs and removed doc-label-missing doc-required Your PR changes impact docs and you will update later. labels May 16, 2024
@aditchawdhary
Copy link
Author

aditchawdhary commented May 16, 2024

hey lhotari, Technoboy-, codelipenghui, gaoran10, congbobo184 and liangyepianzhou

can you take a look?

@lhotari
Copy link
Member

lhotari commented May 16, 2024

hey lhotari, Technoboy-, codelipenghui, gaoran10, congbobo184 and liangyepianzhou

can you take a look?

Great contribution! Thanks.
Please setup Personal CI and run the build in your fork. You'll get feedback about fixing licenses and so on. Since this is a new library, the license will have to be added into the distribution, following the same way as others. The license file would go in distribution/licenses and that would be referenced in distribution/server/src/assemble/LICENSE.bin.txt (footer).

@dao-jun dao-jun added this to the 3.4.0 milestone May 16, 2024
liangyepianzhou
liangyepianzhou requested changes Jun 2, 2024
View reviewed changes
Copy link
Contributor

@liangyepianzhou liangyepianzhou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work! It is a great catch. Leave a little comment.

pulsar-functions/pom.xml
</dependencies>
</dependencyManagement>
<properties>
<versions.java-security-toolkit>1.1.3</versions.java-security-toolkit>
Copy link
Contributor

@liangyepianzhou liangyepianzhou Jun 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the properties in the parent pom file.

@lhotari lhotari modified the milestones: 4.0.0, 4.1.0 Oct 11, 2024
@coderzc coderzc modified the milestones: 4.1.0, 4.2.0 Sep 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@liangyepianzhou liangyepianzhou liangyepianzhou requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@aditchawdhary aditchawdhary

Labels

area/function area/security doc-not-needed Your PR changes do not impact docs release/3.0.16 release/3.2.5

Projects

None yet

Milestone

4.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@aditchawdhary @lhotari @liangyepianzhou @dao-jun @coderzc